fix(mcp): always prompt for the MCP password on interactive http installs - #447
Conversation
…alls An interactive http install could silently skip asking the operator for the shared auth token (MCP password) when one was already sourced from MCP_AUTH_TOKEN env, a flag, or the tunnel collector, writing the agent config with a credential the user never chose or saw. Add an explicit, always-run "MCP Password" step to the install wizard for http/remote installs in interactive mode: it shows whether a token already exists, lets the operator keep it (Enter) or type a replacement (masked), and fails if a public endpoint would be written with no password. Non-interactive installs still source the token from flags/env without prompting.
This comment has been minimized.
This comment has been minimized.
|
SetMCPPassword calls pterm.DefaultInteractiveTextInput...Show() without a timeout or context, which could hang indefinitely during the wizard's ExecuteFunc execution. While this rule targets GORM queries specifically, any GORM calls in the surrounding code should use db.WithContext(ctx) with a timeout to prevent indefinite database operations. Kody rule violation: Disallow GORM queries without timeout |
Code Coverage ReportTotal Coverage: 51.7% Generated from commit: eb43995 |
The MCP Password step updated only the in-memory token used for the agent config's Authorization header. When the operator replaced the password on an install backed by a managed service, the running endpoint kept enforcing the inherited MCP_AUTH_TOKEN from the service env file — so the agent pointed at a credential the server rejected. Now the new password is also persisted to the service env file and mirrored on the service state, keeping the endpoint and the agent config on the same credential. Keeping the existing token needs no propagation.
This comment has been minimized.
This comment has been minimized.
Persisting the new MCP_AUTH_TOKEN to the service env file is not enough on its own: the running endpoint only reads it at process start, so it kept enforcing the old token while the agent config used the new one. Restart the managed MCP service after writing the new token so the live endpoint reloads it and the connection keeps working. The restart seam is wired only in production and only when the install is actually backed by a managed service (--service=false, operator-run servers, and tests skip it).
This comment has been minimized.
This comment has been minimized.
Two correctness fixes to the MCP password change path: - Thread the caller's context through RestartManagedService (and the restart seam) so an interrupted interactive install (Ctrl-C) can cancel the service restart instead of it running on context.Background(). - Order persistAuthToken so a failed restart cannot leave a mismatch: the new token is written to the env file (the restarted process reads it at boot) and only committed to state AFTER the restart succeeds. If the restart fails, the env file and in-memory state are rolled back to the token the still-running endpoint actually enforces, so disk, memory, and the live endpoint agree. Covered by TestMcpInstallHTTPPasswordRestartFailureRollsBack.
This comment has been minimized.
This comment has been minimized.
If the managed service restart fails, the MCP password change rolls the env file back to the token the still-running endpoint enforces. The previous code swallowed the restore-write error, so if the rollback itself could not be persisted the disk file silently kept the uncommitted new password while state used the old one. Now a failed restore write is combined into the returned error instead of masked. Covered by TestMcpInstallHTTPPasswordRestoreFailureSurfaced.
This comment has been minimized.
This comment has been minimized.
| projectDir := t.TempDir() | ||
|
|
||
| envFile := filepath.Join(t.TempDir(), "mcp.env") | ||
| if err := os.WriteFile(envFile, []byte("MCP_AUTH_TOKEN=inherited-token\n"), 0o600); err != nil { |
There was a problem hiding this comment.
Hard-coded secret value 'inherited-token' assigned to MCP_AUTH_TOKEN in mcp_install_test.go violates the rule banning secrets in Go source. Source the token from an environment variable (e.g., os.Getenv) or a secure keystore instead of embedding it directly in test code.
Kody rule violation: Ban hard-coded secrets in Go source
Prompt for LLM
File internal/cli/mcp_install_test.go:
Line 1714:
Hard-coded secret value 'inherited-token' assigned to MCP_AUTH_TOKEN in mcp_install_test.go violates the rule banning secrets in Go source. Source the token from an environment variable (e.g., os.Getenv) or a secure keystore instead of embedding it directly in test code.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
TestMcpInstallHTTPPasswordRestoreFailureSurfaced forces a failed rollback write by making the env file's directory unwritable (os.Chmod on a dir), which Windows does not honor, so on Windows/arm64 the restore write succeeded and the test failed expecting the surfaced restore error. Skip the dir-permission trigger on Windows; the rollback/restore-failure surfacing itself is OS-independent and remains covered on all platforms by TestMcpInstallHTTPPasswordRestartFailureRollsBack.
Kody Review CompleteGreat news! 🎉 Keep up the excellent work! 🚀 Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
An interactive
pinner mcp install --transport httpcould silently skip asking the operator for the shared auth token (the MCP password) when one was already sourced fromMCP_AUTH_TOKENenv, a flag, or the tunnel collector — writing agent config with a credential the user never chose or saw.Adds an explicit, always-run "MCP Password" step for http/remote installs in interactive mode: it shows whether a token already protects the endpoint, lets the operator keep it (Enter) or type a replacement (masked, never echoed), and fails if a public endpoint would be written with no password. Non-interactive installs still source the token from flags/env without prompting.
Summary
This pull request fixes the MCP installation wizard so that it always prompts the operator for the MCP password (the shared auth token protecting a public HTTP endpoint) during interactive HTTP installs, even when a token was already inherited from environment variables, flags, or the tunnel collection step.
Key Changes
1. Always prompt for MCP password in interactive HTTP installs
SetMCPPasswordmethod to theInstallUIinterface and implemented it in thePTermInstallUI(terminal UI).2. Skipping logic
--non-interactive), where the token is sourced from flags/env without prompting.3. Behavior change
4. Tests